feat(acp): add machine stdio transport for Acepe#2858
Conversation
Expose a machine stdio entrypoint in Forge and route it through a real ACP stdio transport so Acepe can launch Forge as an installable provider instead of depending on an unpublished branch. Co-Authored-By: ForgeCode <noreply@forgecode.dev>
| fn acp_start_stdio(&self) -> impl std::future::Future<Output = Result<()>> + Send { | ||
| self.called.store(true, Ordering::SeqCst); | ||
| async { Ok(()) } | ||
| } |
There was a problem hiding this comment.
The test implementation has incorrect async behavior. The self.called.store() executes immediately when the future is created, not when it's awaited. This means the test doesn't accurately verify that the async function is actually executed.
fn acp_start_stdio(&self) -> impl std::future::Future<Output = Result<()>> + Send {
let called = self.called.clone();
async move {
called.store(true, Ordering::SeqCst);
Ok(())
}
}| fn acp_start_stdio(&self) -> impl std::future::Future<Output = Result<()>> + Send { | |
| self.called.store(true, Ordering::SeqCst); | |
| async { Ok(()) } | |
| } | |
| fn acp_start_stdio(&self) -> impl std::future::Future<Output = Result<()>> + Send { | |
| let called = self.called.clone(); | |
| async move { | |
| called.store(true, Ordering::SeqCst); | |
| Ok(()) | |
| } | |
| } | |
Spotted by Graphite
Is this helpful? React 👍 or 👎 to let us know.
- Replace unbounded notification channel with bounded (1024) to apply backpressure when the client stalls - Add per-session model override to prevent concurrent sessions from interfering with each other - Replace From<Error> impl with explicit into_acp_error() per project guidelines - Extract classify_mcp_tool() and convert to free functions, removing the unnecessary ToolOutputConverter struct - Validate MCP server names (length, charset) to prevent injection - Add MAX_BLOB_SIZE (50 MB) guard on base64-decoded resources - Add I/O timeout (5 min) and graceful shutdown drain (5 s) to prevent indefinite hangs - Track cancellation via AtomicBool across loop iterations - Log warnings instead of silently ignoring reload/config errors - Add tests for tool kind mapping, file extraction, and edge cases Co-Authored-By: ForgeCode <noreply@forgecode.dev>
a2e5e58 to
8fe513f
Compare
The store ran eagerly on function call, not when the future was awaited. Move it into the async block so the test actually verifies that the caller awaits the returned future. Co-Authored-By: ForgeCode <noreply@forgecode.dev>
|
Action required: PR inactive for 5 days. |
|
PR closed after 10 days of inactivity. |
|
@amitksingh1490 thanks for looking at this for our user requesting forge integration ! |
|
@flazouh could you please resolve the conflicts would love to integrate this. We are thinking we should introduce a beta feature flag till the time we stablise this and then once its stable expose it to everyone. |
|
Action required: PR inactive for 5 days. |
|
with pleasure, i'm flaze9 on discord |
de35545 to
8db5866
Compare
Co-Authored-By: ForgeCode <noreply@forgecode.dev>
|
Action required: PR inactive for 5 days. |
|
Great work! I am testing the acp feature with the neovim plugin, and I found some critical bugs. I don't know how Acepe works, but it can't work with my neovim plugin. The following are the fixes advised by forge(model deepseek-v4-pro). Outdated Fixes# Summary
|
Extracts independently-mergable fixes from the ongoing ACP work (tailcallhq#2858) that can land on main immediately. Three classes of fix: 1. stdin pre-read encapsulation (cli.rs, main.rs) Add Cli::uses_stdin() method that centralizes the decision of which subcommands own stdin. Subcommands opt in via uses_stdin() rather than having the startup pipeline guess from a hardcoded exclusion list. This makes it easy to add new stdin-consuming subcommands (e.g. an ACP stdio server) without touching the startup logic. 2. Tool output leaking to stdout (context.rs, tool_executor.rs) Shell tool execution streamed raw output to io::stdout(), corrupting the ACP JSON-RPC stream. Add silent: bool field on ToolCallContext (the correct home for per-invocation runtime flags). Default false; ACP sets .silent(true) to route output to io::sink() instead. 3. Stderr vs stdout discipline (main.rs, sandbox.rs) Panic hook and worktree status messages used println! which writes to stdout. Changed to eprintln! (stderr) to avoid contaminating structured transport channels. Co-authored-by: ForgeCode <noreply@forgecode.dev> Co-authored-by: DeepSeek <deepseek@deepseek.com> Signed-off-by: ssfdust <ssfdust@gmail.com>
Extracts independently-mergable fixes from the ongoing ACP work (tailcallhq#2858) that can land on main immediately. Three classes of fix: 1. stdin pre-read encapsulation (cli.rs, main.rs) Add Cli::uses_stdin() method that centralizes the decision of which subcommands own stdin. Subcommands opt in via uses_stdin() rather than having the startup pipeline guess from a hardcoded exclusion list. This makes it easy to add new stdin-consuming subcommands (e.g. an ACP stdio server) without touching the startup logic. 2. Tool output leaking to stdout (context.rs, tool_executor.rs) Shell tool execution streamed raw output to io::stdout(), corrupting the ACP JSON-RPC stream. Add silent: bool field on ToolCallContext (the correct home for per-invocation runtime flags). Default false; ACP sets .silent(true) to route output to io::sink() instead. 3. Stderr vs stdout discipline (main.rs, sandbox.rs) Panic hook and worktree status messages used println! which writes to stdout. Changed to eprintln! (stderr) to avoid contaminating structured transport channels. Signed-off-by: ssfdust <ssfdust@gmail.com> Co-authored-by: ForgeCode <noreply@forgecode.dev> Co-authored-by: DeepSeek <deepseek@deepseek.com>
Extracts independently-mergable fixes from the ongoing ACP work (tailcallhq#2858) that can land on main immediately. Three classes of fix: 1. stdin pre-read encapsulation (cli.rs, main.rs) Add Cli::uses_stdin() method that centralizes the decision of which subcommands own stdin. Subcommands opt in via uses_stdin() rather than having the startup pipeline guess from a hardcoded exclusion list. This makes it easy to add new stdin-consuming subcommands (e.g. an ACP stdio server) without touching the startup logic. 2. Tool output leaking to stdout (context.rs, tool_executor.rs) Shell tool execution streamed raw output to io::stdout(), corrupting the ACP JSON-RPC stream. Add silent: bool field on ToolCallContext (the correct home for per-invocation runtime flags). Default false; ACP sets .silent(true) to route output to io::sink() instead. 3. Stderr vs stdout discipline (main.rs, sandbox.rs) Panic hook and worktree status messages used println! which writes to stdout. Changed to eprintln! (stderr) to avoid contaminating structured transport channels. Signed-off-by: ssfdust <ssfdust@gmail.com> Co-authored-by: ForgeCode <noreply@forgecode.dev> Co-authored-by: DeepSeek <deepseek-ai@deepseek.com>
Extracts independently-mergable fixes from the ongoing ACP work (tailcallhq#2858) that can land on main immediately. Three classes of fix: 1. stdin pre-read encapsulation (cli.rs, main.rs) Add Cli::uses_stdin() method that centralizes the decision of which subcommands own stdin. Subcommands opt in via uses_stdin() rather than having the startup pipeline guess from a hardcoded exclusion list. This makes it easy to add new stdin-consuming subcommands (e.g. an ACP stdio server) without touching the startup logic. 2. Tool output leaking to stdout (context.rs, tool_executor.rs) Shell tool execution streamed raw output to io::stdout(), corrupting the ACP JSON-RPC stream. Add silent: bool field on ToolCallContext (the correct home for per-invocation runtime flags). Default false; ACP sets .silent(true) to route output to io::sink() instead. 3. Stderr vs stdout discipline (main.rs, sandbox.rs) Panic hook and worktree status messages used println! which writes to stdout. Changed to eprintln! (stderr) to avoid contaminating structured transport channels. Signed-off-by: ssfdust <ssfdust@gmail.com> Co-authored-by: ForgeCode <noreply@forgecode.dev> Co-authored-by: DeepSeek <deepseek-ai@deepseek.com>
Extracts independently-mergable fixes from the ongoing ACP work (tailcallhq#2858) that can land on main immediately. Three classes of fix: 1. stdin pre-read encapsulation (cli.rs, main.rs) Add Cli::uses_stdin() method that centralizes the decision of which subcommands own stdin. Subcommands opt in via uses_stdin() rather than having the startup pipeline guess from a hardcoded exclusion list. This makes it easy to add new stdin-consuming subcommands (e.g. an ACP stdio server) without touching the startup logic. 2. Tool output leaking to stdout (context.rs, tool_executor.rs) Shell tool execution streamed raw output to io::stdout(), corrupting the ACP JSON-RPC stream. Add silent: bool field on ToolCallContext (the correct home for per-invocation runtime flags). Default false; ACP sets .silent(true) to route output to io::sink() instead. 3. Stderr vs stdout discipline (main.rs, sandbox.rs) Panic hook and worktree status messages used println! which writes to stdout. Changed to eprintln! (stderr) to avoid contaminating structured transport channels. Signed-off-by: ssfdust <ssfdust@gmail.com> Co-authored-by: ForgeCode <noreply@forgecode.dev> Co-authored-by: DeepSeek <deepseek-ai@deepseek.com>
Extracts independently-mergable fixes from the ongoing ACP work (tailcallhq#2858) that can land on main immediately. Three classes of fix: 1. stdin pre-read encapsulation (cli.rs, main.rs) Add Cli::uses_stdin() method that centralizes the decision of which subcommands own stdin. Subcommands opt in via uses_stdin() rather than having the startup pipeline guess from a hardcoded exclusion list. This makes it easy to add new stdin-consuming subcommands (e.g. an ACP stdio server) without touching the startup logic. 2. Tool output leaking to stdout (context.rs, tool_executor.rs) Shell tool execution streamed raw output to io::stdout(), corrupting the ACP JSON-RPC stream. Add silent: bool field on ToolCallContext (the correct home for per-invocation runtime flags). Default false; ACP sets .silent(true) to route output to io::sink() instead. 3. Stderr vs stdout discipline (main.rs, sandbox.rs) Panic hook and worktree status messages used println! which writes to stdout. Changed to eprintln! (stderr) to avoid contaminating structured transport channels. Signed-off-by: ssfdust <ssfdust@gmail.com> Co-authored-by: ForgeCode <noreply@forgecode.dev> Co-authored-by: DeepSeek <deepseek-ai@deepseek.com>
|
Awesome to see you are working on it. Opencode's command is :opencode acp |
Extracts independently-mergable fixes from the ongoing ACP work (tailcallhq#2858) that can land on main immediately. Three classes of fix: 1. stdin pre-read encapsulation (cli.rs, main.rs) Add Cli::uses_stdin() method that centralizes the decision of which subcommands own stdin. Subcommands opt in via uses_stdin() rather than having the startup pipeline guess from a hardcoded exclusion list. This makes it easy to add new stdin-consuming subcommands (e.g. an ACP stdio server) without touching the startup logic. 2. Tool output leaking to stdout (context.rs, tool_executor.rs) Shell tool execution streamed raw output to io::stdout(), corrupting the ACP JSON-RPC stream. Add silent: bool field on ToolCallContext (the correct home for per-invocation runtime flags). Default false; ACP sets .silent(true) to route output to io::sink() instead. 3. Stderr vs stdout discipline (main.rs, sandbox.rs) Panic hook and worktree status messages used println! which writes to stdout. Changed to eprintln! (stderr) to avoid contaminating structured transport channels. Signed-off-by: ssfdust <ssfdust@gmail.com> Co-authored-by: ForgeCode <noreply@forgecode.dev> Co-authored-by: DeepSeek <deepseek-ai@deepseek.com>
Extracts independently-mergable fixes from the ongoing ACP work (tailcallhq#2858) that can land on main immediately. Three classes of fix: 1. stdin pre-read encapsulation (cli.rs, main.rs) Add Cli::uses_stdin() method that centralizes the decision of which subcommands own stdin. Subcommands opt in via uses_stdin() rather than having the startup pipeline guess from a hardcoded exclusion list. This makes it easy to add new stdin-consuming subcommands (e.g. an ACP stdio server) without touching the startup logic. 2. Tool output leaking to stdout (context.rs, tool_executor.rs) Shell tool execution streamed raw output to io::stdout(), corrupting the ACP JSON-RPC stream. Add silent: bool field on ToolCallContext (the correct home for per-invocation runtime flags). Default false; ACP sets .silent(true) to route output to io::sink() instead. 3. Stderr vs stdout discipline (main.rs, sandbox.rs) Panic hook and worktree status messages used println! which writes to stdout. Changed to eprintln! (stderr) to avoid contaminating structured transport channels. Signed-off-by: ssfdust <ssfdust@gmail.com> Co-authored-by: ForgeCode <noreply@forgecode.dev> Co-authored-by: DeepSeek <deepseek-ai@deepseek.com>
|
Any advances on this? Seems like this got stale, |
| use super::error::{Error, Result}; | ||
|
|
||
| /// Maximum number of buffered session notifications before backpressure. | ||
| const NOTIFICATION_CHANNEL_CAPACITY: usize = 1024; |
There was a problem hiding this comment.
move this to config
| app.execute(data_parameters).await | ||
| } | ||
|
|
||
| async fn acp_start_stdio(&self) -> Result<()> { |
There was a problem hiding this comment.
This should not be part of api, instead it should be build on top of api
|
Action required: PR inactive for 5 days. |
|
Will address those comments ASAP, guys ! |
Abstract
This PR adds a real
forge machine stdioentrypoint backed by Forge's ACP stdio transport. It is primarily for our app, Acepe, which needs to launch Forge from a stable CLI surface without depending on an unpublished feature branch.Product Context
Acepe is our desktop app for running and managing coding agents across providers and protocols.
Product links:
Problem
Acepe integrates Forge as an installable local provider. That integration needed a released Forge command that can:
Before this change, the relevant command path was missing on
main. We could add parser coverage forforge machine stdio, but that would still leave the command non-functional at runtime, which does not unblock Acepe.Solution
This change wires the machine-facing CLI entrypoint all the way through to a working ACP stdio server:
machine stdioas a first-class top-level CLI commandforge_maininto a dedicated stdio runneracp_start_stdio()so the transport can be started through the current app/service stackforge_appThe intent is to keep the public invocation simple while making the underlying server path real and launchable by external applications.
ASCII Schemas
Before
After
Ownership Boundary
Why This Is For Acepe
Acepe needs Forge to expose a stable, installable machine transport from
mainso our desktop app can launch Forge directly as a provider. This PR moves that capability into the open-source Forge repo so Acepe no longer has to depend on branch-specific or unpublished transport work.Files Of Interest
crates/forge_main/src/cli.rscrates/forge_main/src/ui.rscrates/forge_main/src/acp_runner.rscrates/forge_api/src/api.rscrates/forge_api/src/forge_api.rscrates/forge_app/src/acp_app.rscrates/forge_app/src/acp/Verification
I verified the change with:
Notes
protocwas not available on PATH in the local environment, so verification used a staged temporary binary.